[Refactor] pagination 상태 유지 개선(searchParams)#158
Conversation
- useSearchParams 적용하여 뒤로가기 시에도 검색어와 페이지 유지되도록 수정 - useEffect 통해 검색어 유지되도록 수정
📝 WalkthroughWalkthroughExperience 타입을 유니온에서 문자열로 완화하고 관련 컴포넌트/쿼리 타입을 문자열 기반으로 업데이트했으며, 여러 페이지 컴포넌트에서 로컬 상태 대신 React Router의 URL 검색 파라미터(useSearchParams)로 필터와 페이지 상태 관리를 마이그레이션했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant 사용자 as 사용자
participant 페이지 as 페이지 컴포넌트
participant 라우터 as React Router (useSearchParams)
participant 쿼리 as API 훅 (useGetExperienceList)
participant 서버 as 서버/API
사용자->>페이지: 방문 또는 필터/검색 변경
페이지->>라우터: searchParams 읽음/갱신
라우터-->>페이지: 현재 쿼리 파라미터 반환 (type, page, keyword ...)
페이지->>쿼리: useGetExperienceList(type, page, ...)
쿼리->>서버: HTTP 요청 (type은 string)
서버-->>쿼리: 응답 데이터
쿼리-->>페이지: 데이터 반환
페이지->>사용자: UI 렌더링 (목록, 페이징, 필터 상태)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
🚀 빌드 결과✅ 린트 검사 완료 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/features/experience/api/use-experience-list.query.ts (1)
29-32:⚠️ Potential issue | 🟠 Major
queryKey와queryFn에서 falsy 값 처리가 불일치해요.queryKey: experienceQueryKey.list(type ?? "", page), // "" 으로 정규화 queryFn: () => getExperienceList({ type: type ?? undefined, page }), // undefined로 정규화이로 인해 캐시 동작에 문제가 발생할 수 있어요:
type=null,type="",type=undefined가 모두 같은 캐시 키("")를 공유하지만- 실제 API 호출은 다르게 동작해요 (
""vsundefined쿼리 파라미터)캐시 키와 API 호출이 일관되게 동작하도록 통일해주세요.
🔧 일관된 처리를 위한 제안
export const useGetExperienceList = ({ type, page, }: { type?: string | null; page: number; }) => { + const normalizedType = type || undefined; return useQuery({ - queryKey: experienceQueryKey.list(type ?? "", page), - queryFn: () => getExperienceList({ type: type ?? undefined, page }), + queryKey: experienceQueryKey.list(normalizedType ?? "", page), + queryFn: () => getExperienceList({ type: normalizedType, page }), }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/experience/api/use-experience-list.query.ts` around lines 29 - 32, Normalize the `type` value the same way in both the cache key and the API call: choose a single canonical form (e.g., undefined) and apply it to both `experienceQueryKey.list(...)` and the call to `getExperienceList(...)` so the cache key matches the actual request parameters; specifically, update the `queryKey` invocation (experienceQueryKey.list) to accept the same normalized `type` as the `queryFn` that calls `getExperienceList` (both using type ?? undefined or both using type ?? ""), ensuring consistency between the cache key and the API request.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/experience/experience-page.tsx`:
- Around line 34-39: handlePageChange currently uses an unnecessary nullish
coalescing (type ?? "") even though type is already derived from
searchParams.get("type") || "" and thus always a string; update the
handlePageChange function to pass type directly to setSearchParams (remove the
"?? \"\"" usage) so setSearchParams({ type: type, page: String(page) }) uses the
existing type variable from the searchParams.get("type") || "" initialization.
In `@src/widgets/experience-filter/experience-filter.tsx`:
- Line 22: Guard the cast of value to ExperienceTypeCode before indexing
EXPERIENCE_TYPE: replace the unvalidated cast in the experience-filter component
by checking membership first (e.g., if (value &&
Object.prototype.hasOwnProperty.call(EXPERIENCE_TYPE, value)) triggerLabel =
EXPERIENCE_TYPE[value as ExperienceTypeCode];) so that triggerLabel cannot
become undefined when an invalid string is passed; reference the symbols value,
EXPERIENCE_TYPE, ExperienceTypeCode, and triggerLabel in your change.
---
Outside diff comments:
In `@src/features/experience/api/use-experience-list.query.ts`:
- Around line 29-32: Normalize the `type` value the same way in both the cache
key and the API call: choose a single canonical form (e.g., undefined) and apply
it to both `experienceQueryKey.list(...)` and the call to
`getExperienceList(...)` so the cache key matches the actual request parameters;
specifically, update the `queryKey` invocation (experienceQueryKey.list) to
accept the same normalized `type` as the `queryFn` that calls
`getExperienceList` (both using type ?? undefined or both using type ?? ""),
ensuring consistency between the cache key and the API request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0cdcf287-49f4-4da2-a5be-0ec5d6d95d1c
📒 Files selected for processing (8)
src/features/experience-detail/types/experience-detail.types.tssrc/features/experience-detail/ui/experience-viewer/experience-viewer.tsxsrc/features/experience/api/use-experience-list.query.tssrc/pages/experience/experience-page.tsxsrc/pages/home/search-section/search-section.tsxsrc/pages/matching-list/matching-list-page.tsxsrc/shared/api/generate/http-client.tssrc/widgets/experience-filter/experience-filter.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/experience/experience-page.tsx`:
- Line 17: The code forces searchParams.get("type") to an empty string which
breaks the "no-filter selected" state; revert to letting const type =
searchParams.get("type") return null when absent and update any places that read
or set this param (the var name type and uses of searchParams.get) to treat null
as "unset" — when updating the URL/query (e.g., via your setSearchParams or
related hooks referenced around lines 22-23, 28-31, 35-37) remove the key
entirely instead of writing type=""; ensure any consumer hooks accept
null/undefined as the unset state rather than an empty string.
- Line 16: The page query isn't being normalized so values like "-1" or "1.5"
slip through; read the raw value from searchParams.get("page"), parse it with
parseInt(raw, 10) (or Math.floor(Number(raw))), validate that the parsed value
is an integer >= 1, and if it's NaN or < 1 fall back to 1; replace the current
currentPage assignment (which uses Number(searchParams.get("page")) || 1) with
this defensive parsing/validation logic so currentPage is always a positive
integer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 31c762cb-a7b0-44b0-b011-746c1ef767a1
📒 Files selected for processing (1)
src/pages/experience/experience-page.tsx
| const navigate = useNavigate(); | ||
| const [searchParams, setSearchParams] = useSearchParams(); | ||
|
|
||
| const currentPage = Number(searchParams.get("page")) || 1; |
There was a problem hiding this comment.
page 쿼리 파라미터 정규화가 없어 잘못된 페이지 요청이 가능합니다.
현재 계산식은 -1, 1.5 같은 값도 그대로 통과합니다. 페이지는 1 이상의 정수로 제한해 방어적으로 처리하는 게 안전합니다.
🔧 수정 제안
- const currentPage = Number(searchParams.get("page")) || 1;
+ const parsedPage = Number(searchParams.get("page"));
+ const currentPage =
+ Number.isInteger(parsedPage) && parsedPage > 0 ? parsedPage : 1;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const currentPage = Number(searchParams.get("page")) || 1; | |
| const parsedPage = Number(searchParams.get("page")); | |
| const currentPage = | |
| Number.isInteger(parsedPage) && parsedPage > 0 ? parsedPage : 1; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/experience/experience-page.tsx` at line 16, The page query isn't
being normalized so values like "-1" or "1.5" slip through; read the raw value
from searchParams.get("page"), parse it with parseInt(raw, 10) (or
Math.floor(Number(raw))), validate that the parsed value is an integer >= 1, and
if it's NaN or < 1 fall back to 1; replace the current currentPage assignment
(which uses Number(searchParams.get("page")) || 1) with this defensive
parsing/validation logic so currentPage is always a positive integer.
| const [searchParams, setSearchParams] = useSearchParams(); | ||
|
|
||
| const currentPage = Number(searchParams.get("page")) || 1; | ||
| const type = searchParams.get("type") || ""; |
There was a problem hiding this comment.
type를 빈 문자열로 강제하면 “필터 미선택” 상태가 깨집니다.
Line 17에서 null을 ""로 바꿔버려서, Line 22의 훅으로 type=""이 전달됩니다. 이러면 필터가 없는 상태여도 API/URL에 빈 값(type=)이 남을 수 있습니다. null을 유지하고, 값이 없을 때는 쿼리 키를 삭제하는 방식이 안전합니다.
🔧 수정 제안
- const type = searchParams.get("type") || "";
+ const type = searchParams.get("type");
const { data } = useGetExperienceList({
type,
page: currentPage,
});
const handleFilterChange = (value: string) => {
setIsExpTouched(true);
- setSearchParams({
- type: value,
- page: "1",
- });
+ setSearchParams((prev) => {
+ const next = new URLSearchParams(prev);
+ if (value) next.set("type", value);
+ else next.delete("type");
+ next.set("page", "1");
+ return next;
+ });
};
const handlePageChange = (page: number) => {
- setSearchParams({
- type,
- page: String(page),
- });
+ setSearchParams((prev) => {
+ const next = new URLSearchParams(prev);
+ if (type) next.set("type", type);
+ else next.delete("type");
+ next.set("page", String(page));
+ return next;
+ });
};Also applies to: 22-23, 28-31, 35-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/experience/experience-page.tsx` at line 17, The code forces
searchParams.get("type") to an empty string which breaks the "no-filter
selected" state; revert to letting const type = searchParams.get("type") return
null when absent and update any places that read or set this param (the var name
type and uses of searchParams.get) to treat null as "unset" — when updating the
URL/query (e.g., via your setSearchParams or related hooks referenced around
lines 22-23, 28-31, 35-37) remove the key entirely instead of writing type="";
ensure any consumer hooks accept null/undefined as the unset state rather than
an empty string.
✏️ Summary
📑 Tasks
페이지네이션의 상태가 유지되도록 리팩토링을 진행했습니다. 상세 페이지에서 '뒤로가기' 버튼을 통해 목록 페이지로 넘어왔을 때 이전 상태를 유지하려면 url에 목록에 관련된 정보들을 저장해야 하는데요, 이 때 페이지와 필터, 검색어 등 상태가 유지되어야 하는 정보들을 쿼리 파라미터로 넘겨 페이지를 옮겨다니더라도 상태가 유지되도록 구현해주었습니다.
구현 방식은 위와 같습니다. 페이지 렌더링 시
searchParams를 통해 쿼리 파라미터의 값을 가져오고, 존재하지 않을 경우 디폴트 값으로 설정되도록 수정해주었습니다. 추가적으로 인라인 함수로 구현되어 있던 부분도 컨벤션에 맞게 'handle-' 형태의 함수로 분리해주었습니다.현재 페이지네이션이 사용되는 페이지는 홈, 경험 목록, 매칭 경험 목록 이렇게 3가지인데요, 이중 홈과 매칭 경험 목록의 경우 검색 기능이 포함되어 있는데 검색값이 날아가는 것을 방지하기 위해 useEffect를 통해 검색값이 유지되도록 해주었습니다~
👀 To Reviewer
리팩토링 과정에서 ExperienceFilter 컴포넌트의 value의 type을
string | null로 확장해주었습니다. 그 이유는 .. 제가 안정성을 위해 범위를 너무 좁혀놨더니 오히려 결합도가 지나치게 올라가서 하나 수정하려고 130913개의 파일을 건너야 하는 불상사가 발생하여서 리팩토링 하는 김에 같이 바꿔주었습니다!이 부분 수정하면서 http-client의 getRequestDto와, ExperienceRequestDto의
type과 getSummaryExperienceList 메소드의type의 타입을 string으로 수정해주었습니다. 또한 유진이가 작업한 파일에서이렇게 수정되었습니다! 타입이랑, label의 기본값 정도만 수정되어서 큰 문제는 없겠지만 그래도 알아두어야 할 거 같아 전달합니다 ~
📸 Screenshot
추추의 작업 이슈로 화면녹화를 못하네요 쩝 .. 테스트 직접 해보기 ~~~~ >_<
🔔 ETC
Summary by CodeRabbit
릴리스 노트
버그 수정
리팩토링
변경